Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Updated deprecated CPanel calls #2184

Merged
merged 10 commits into from
Jan 21, 2025
Merged

Updated deprecated CPanel calls #2184

merged 10 commits into from
Jan 21, 2025

Conversation

RubisetCie
Copy link
Contributor

Hello everyone, I hope you're doing fine. :)

I've noticed base Garry's Mod uses the AddControl function for the UI panel creation.

To quote Garry himself in here:

You shouldn't ever really be calling AddControl.
If you're writing new code - don't call AddControl!! Add stuff directly using the DForm member functions!

Thus, I took the time to update all the AddControl calls with their non-deprecated counterparts (moreover, it should improve performance).

I did an extensive testing to make sure it doesn't break anything, and it works flawlessly (but of course, feel free to check, I may have made mistakes ;).

robotboy655
robotboy655 requested changes Jan 13, 2025
View reviewed changes
Copy link
Collaborator

@robotboy655 robotboy655 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You really should not have done everything in 1 go. I'd encourage you to move the Sandbox tool changes to a separate Pull Request. It makes reviewing changes easier.

That being said, this PR has plenty of issues that needs addressing before it can be merged.

garrysmod/gamemodes/sandbox/entities/weapons/gmod_tool/stools/editentity.lua Outdated Show resolved Hide resolved
garrysmod/gamemodes/sandbox/entities/weapons/gmod_tool/stools/axis.lua Outdated Show resolved Hide resolved
garrysmod/gamemodes/sandbox/entities/weapons/gmod_tool/stools/axis.lua Outdated Show resolved Hide resolved
garrysmod/gamemodes/sandbox/entities/weapons/gmod_tool/stools/axis.lua Outdated Show resolved Hide resolved
garrysmod/gamemodes/sandbox/entities/weapons/gmod_tool/stools/button.lua Outdated Show resolved Hide resolved
garrysmod/gamemodes/sandbox/entities/weapons/gmod_tool/stools/button.lua Outdated Show resolved Hide resolved
garrysmod/gamemodes/sandbox/entities/weapons/gmod_tool/stools/button.lua Outdated Show resolved Hide resolved
garrysmod/gamemodes/sandbox/entities/weapons/gmod_tool/stools/duplicator.lua Outdated Show resolved Hide resolved
garrysmod/gamemodes/sandbox/entities/weapons/gmod_tool/stools/material.lua Outdated Show resolved Hide resolved
garrysmod/lua/postprocess/color_modify.lua Outdated Show resolved Hide resolved
@robotboy655 robotboy655 added the Enhancement The pull request enhances current functionality. label Jan 13, 2025
@RubisetCie
Copy link
Contributor Author

You really should not have done everything in 1 go. I'd encourage you to move the Sandbox tool changes to a separate Pull Request. It makes reviewing changes easier.

Understood. I'll concentrate this very PR on the Sandbox tool changes for now.

That being said, this PR has plenty of issues that needs addressing before it can be merged.

Thank you for the return, I'll proceed to the changes tomorrow.

@RubisetCie
Copy link
Contributor Author

Hi @robotboy655, I've made some changes to the PR, according to what you advised:

  • This PR is limited to the sandbox tools.
  • Added a function PANEL:PropSelect which encapsulate the prop selection UI.
  • Automatically set the default value of DNumSlider to be the default value of the associated convar.

It should be better now, feel free to review and suggest more modifications. :)

robotboy655
robotboy655 requested changes Jan 17, 2025
View reviewed changes
Copy link
Collaborator

@robotboy655 robotboy655 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's a tip:
Use the "Resolve Conversation" buttons on the requested changes, so you and me both can keep track of what was addressed and what wasn't yet. It also reduces the height of this web page.

There also should be a button to request review once you have done all the changes you wanted to.

garrysmod/gamemodes/sandbox/entities/weapons/gmod_tool/stools/eyeposer.lua Outdated Show resolved Hide resolved
garrysmod/gamemodes/sandbox/entities/weapons/gmod_tool/stools/camera.lua Outdated Show resolved Hide resolved
garrysmod/gamemodes/sandbox/entities/weapons/gmod_tool/stools/balloon.lua Outdated Show resolved Hide resolved
garrysmod/gamemodes/sandbox/entities/weapons/gmod_tool/stools/eyeposer.lua Outdated Show resolved Hide resolved
garrysmod/gamemodes/sandbox/entities/weapons/gmod_tool/stools/faceposer.lua Outdated Show resolved Hide resolved
garrysmod/gamemodes/sandbox/entities/weapons/gmod_tool/stools/muscle.lua Outdated Show resolved Hide resolved
garrysmod/gamemodes/sandbox/gamemode/spawnmenu/controlpanel.lua Outdated Show resolved Hide resolved
garrysmod/gamemodes/sandbox/entities/weapons/gmod_tool/stools/pulley.lua Outdated Show resolved Hide resolved
garrysmod/gamemodes/sandbox/entities/weapons/gmod_tool/stools/colour.lua Outdated Show resolved Hide resolved
@RubisetCie
Copy link
Contributor Author

Hello @robotboy655 . I have pushed the changes you requested, and marked the points as resolved.

Don't hesitate to suggest more changes.

robotboy655
robotboy655 approved these changes Jan 21, 2025
View reviewed changes
Copy link
Collaborator

@robotboy655 robotboy655 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, I have fixed up the remaining issues, seems to be good now.

@robotboy655 robotboy655 merged commit 076561d into Facepunch:master Jan 21, 2025
@RubisetCie
Copy link
Contributor Author

Thank you very much for your time and your professionalism!

As you advised, I'll do the second part of the "deprecation-update" in a separate pull request, in the next days.
I'll try my best to apply your suggestions the first time.

@RubisetCie RubisetCie mentioned this pull request Jan 24, 2025
Merged
@RubisetCie RubisetCie deleted the deprecation-update branch January 29, 2025 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robotboy655 robotboy655 robotboy655 approved these changes

Assignees
No one assigned
Labels
Enhancement The pull request enhances current functionality.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@RubisetCie @robotboy655